Detection infrastructure rework: Secondary detection stream, In-process LiteRT engine, Configurable grace period#443
Merged
Conversation
Add a per-stream detection_url column: an optional secondary stream (e.g. a low-res MJPEG sub-stream) used exclusively for object detection, leaving the main stream for buffering/recording. - migration 0042 adds the detection_url TEXT column (filesystem + embedded) - db_streams.c loads/saves it alongside the other stream fields - stream_config_t gains the in-memory detection_url field Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- POST/PUT stream handlers parse, validate (http/https/rtsp/rtsps only, blocking file://, concat:, etc.) and persist detection_url, and treat it as a restart-triggering field. - GET stream endpoints emit detection_url so the UI can round-trip it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ection stream Restructure the unified detection thread into a clean producer/consumer pipeline and add an optional secondary detection stream: - When a stream's detection_url is set, a dedicated producer thread opens that URL (network protocols only), decodes frames, and runs the configured model; the main thread consumes the result and drives the recording state machine. Detection falls back to the main stream whenever the secondary stream is connecting/reconnecting. - Split the pipeline into detect → report_detections → handle_recording_state. - Intra-only (e.g. MJPEG) detection streams skip redundant decodes via the codec descriptor's INTRA_ONLY property. - current_recording_id is _Atomic for the cross-thread (producer/consumer) read. - Recording robustness: carry pre_buffer_seconds into the MP4 writer and warn on implausible forward DTS jumps before the monotonicity fixup. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add the optional "Detection Stream URL" field to the stream config modal (create/edit/clone round-trip via the detection_url payload key) and the streamsConfig.detectionUrl* strings to all 17 locales. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the hardcoded DETECTION_GRACE_PERIOD_SEC with a configurable [detection] grace_period (0–60s, default 2), read from the config file and applied by the unified detection thread. The RECORDING→POST_BUFFER transition now waits detection_interval + grace_period before entering post-buffer: detection is sampled only once per interval, so without that margin a continuously-active scene would fragment into multiple clips. The retroactive detection-linking lookback uses the same window for consistency. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add the grace-period control to the Detection settings tab (round-tripped through api_handlers_settings.c as detection_grace_period, clamped 0–60) and the settings.detectionGracePeriod* strings to all 17 locales. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Capture the wall-clock time when a frame enters the detection pipeline and thread it through run_detection_on_frame() into detect_objects_api / detect_objects_api_snapshot, so detections.timestamp (and the MQTT event) reflect frame time rather than inference-completion time. ONVIF captures its timestamp before the PullMessages roundtrip. The DB layer still falls back to time(NULL) when callers pass 0, preserving the legacy detection.c stop-gap caller. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the dead libtensorflowlite.so dlopen stub with a real in-process
LiteRT engine. Streams with a .tflite model_path flow through the existing
detection_model_t / detect_objects dispatch — no UDT sentinel.
- src/video/detection/litert_engine.{h,cc}: opaque C ABI + refcounted
per-model registry (engines shared across streams using the same .tflite,
mutex-serialized). Data-driven input dims/dtype (float32/uint8/int8 with
per-tensor quantization), letterbox via libswscale, end-to-end YOLO
[1,N,6] output enforced at init. Labels from a <basename>.labels.txt
sidecar with class_<n> fallback.
- CMake: ENABLE_LITERT (default OFF), LITERT_WITH_XNNPACK/_GPU; conditional
add_subdirectory of the vendored LiteRT submodule + tensorflow-lite link
+ HAVE_LITERT. litert_engine.cc pinned to C++17.
- detection_model reshaped to hold the opaque engine; threshold is per-call.
- New [detection_engine] INI section (enabled/threads/delegate) with a
matching DetectionTab subsection, round-tripped via api_handlers_settings.
- Hard stream error on model-load failure: one-shot handle_stream_error
(STREAM_ERR_MODEL_LOAD) instead of swallow-and-retry; last_error_message
surfaced as streams.error_message and rendered as a StreamCard tooltip.
- All 17 locales gain settings.detectionEngine* and streams.errorModelLoad.
- Embedded migration count includes 0042; engine migration regenerated.
LiteRT vendored as a git submodule under third_party/litert.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The detector-memory stat scanned /proc for an external light-object-detect process; with the in-process LiteRT engine there is none, so it always read 0. - litert_engine tracks a running total of all loaded engines' memory (model flatbuffer + a sum of non-weight tensor buffers, captured at init), maintained on engine create/destroy and exposed via a lock-free litert_engine_registry_memory_bytes() — the reader never touches a live interpreter, so it can't race a multi-threaded Invoke(). - The system API adds that footprint to detectorMemory and subtracts it from the lightnvr process figure, since the engine lives inside this process — otherwise the UI, which sums the two, would double-count it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
handle_post_settings applied every setting to g_config in place and then re-read the config from disk "to ensure changes are applied". That reload ran load_default_config(), which free()s the heap-allocated config.streams array — on the libuv worker thread serving the request, while the main thread is concurrently reading config.streams[i] (e.g. in stop_detection_stream_reader during shutdown/stream teardown). That is a heap use-after-free (confirmed by AddressSanitizer) and matches earlier SIGSEGV cores in main()'s shutdown loop. The reload was redundant: the settings are already in g_config and persisted by save_config(). Drop it. Settings that need more than an in-memory update (max_streams) already set restart_required and take effect on restart. This leaves reload_config() with no callers, so remove it (config.c + config.h). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The unified detection thread sets log_set_thread_context("Detection",
stream_name), so the logger already prefixes "[Detection] [stream]". Many
messages also prepend the name manually via "[%s]", producing
"[Detection] [stream] [stream] ...". Set the thread context to the component
only and let each message's "[%s]" supply the stream name. (Pre-existing;
present since before the detection rework.)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
|
✅ All contributors have signed the CLA. Thank you! |
Contributor
Author
|
I have read the CLA Document and I hereby sign the CLA |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR reworks LightNVR’s detection pipeline to support an optional secondary per-stream “detection-only” input, adds an in-process LiteRT/TFLite inference engine, improves detection timestamping (frame-arrival time), and makes the detection grace period configurable across the API/UI/config.
Changes:
- Add
detection_urlas an optional secondary stream for object detection (DB migration + API + UI + UDT producer/consumer pipeline). - Vendor and integrate an in-process LiteRT/TFLite engine with configurable threads/delegate and improved memory reporting.
- Add configurable detection grace period and propagate frame-arrival timestamps into detection/recording paths.
Reviewed changes
Copilot reviewed 51 out of 51 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| web/public/locales/ar.json | Add localized strings for detection URL, LiteRT engine settings, grace period |
| web/public/locales/de.json | Add localized strings for detection URL, LiteRT engine settings, grace period |
| web/public/locales/en.json | Add localized strings for detection URL, LiteRT engine settings, grace period |
| web/public/locales/es.json | Add localized strings for detection URL, LiteRT engine settings, grace period |
| web/public/locales/fr.json | Add localized strings for detection URL, LiteRT engine settings, grace period |
| web/public/locales/it.json | Add localized strings for detection URL, LiteRT engine settings, grace period |
| web/public/locales/ja.json | Add localized strings for detection URL, LiteRT engine settings, grace period |
| web/public/locales/ko.json | Add localized strings for detection URL, LiteRT engine settings, grace period |
| web/public/locales/nl.json | Add localized strings for detection URL, LiteRT engine settings, grace period |
| web/public/locales/pl.json | Add localized strings for detection URL, LiteRT engine settings, grace period |
| web/public/locales/pt-BR.json | Add localized strings for detection URL, LiteRT engine settings, grace period |
| web/public/locales/pt-PT.json | Add localized strings for detection URL, LiteRT engine settings, grace period |
| web/public/locales/ru.json | Add localized strings for detection URL, LiteRT engine settings, grace period |
| web/public/locales/tr.json | Add localized strings for detection URL, LiteRT engine settings, grace period |
| web/public/locales/uk.json | Add localized strings for detection URL, LiteRT engine settings, grace period |
| web/public/locales/zh.json | Add localized strings for detection URL, LiteRT engine settings, grace period |
| web/public/locales/zh-TW.json | Add localized strings for detection URL, LiteRT engine settings, grace period |
| web/js/components/preact/StreamsView.jsx | Add detectionUrl to stream form state and API payloads |
| web/js/components/preact/StreamConfigModal.jsx | Add “Detection Stream URL” input to stream config modal |
| web/js/components/preact/StreamCard.jsx | Show stream error cause via tooltip (error_message) |
| web/js/components/preact/SettingsView.jsx | Round-trip detection grace period + LiteRT engine settings via settings API |
| web/js/components/preact/settings/DetectionTab.jsx | Add UI controls for grace period and LiteRT engine runtime knobs |
| tests/unit/test_api_detection.c | Update API detection function signature in unit test |
| src/web/api_handlers_system.c | Subtract LiteRT in-process footprint from process RSS and report separately |
| src/web/api_handlers_streams_modify.c | Parse/validate/persist detection_url and trigger restarts on change |
| src/web/api_handlers_streams_get.c | Return detection_url and expose error_message in stream GET responses |
| src/web/api_handlers_settings.c | Add settings API fields for grace period and LiteRT engine configuration |
| src/web/api_handlers_detection_models.c | Filter detection model list to only runnable/supported model files |
| src/video/unified_detection_thread.c | Producer/consumer detection stream thread, grace window config, frame-time stamping |
| src/video/stream_state.c | Persist a human-readable last error message and refine reconnect logic |
| src/video/onvif_detection.c | Use pre-roundtrip timestamps for ONVIF motion event storage/publishing |
| src/video/mp4_segment_recorder.c | Add DTS forward-gap detection to warn on discontinuous MP4 timelines |
| src/video/detection/litert_engine.cc | Implement in-process LiteRT/TFLite engine + registry + delegates |
| src/video/detection.c | Route .tflite detection through LiteRT when compiled in |
| src/video/detection_model.c | Replace dlopen-based TFLite probe with vendored LiteRT integration |
| src/database/db_streams.c | Persist/load detection_url in stream config DB operations |
| src/core/config.c | Add grace period + LiteRT config defaults, parsing, setters, and persistence |
| include/video/unified_detection_thread.h | Add detection stream thread fields + atomic recording id |
| include/video/stream_state.h | Add model-load error code and last-error-message storage |
| include/video/mp4_writer.h | Add pre_buffer_seconds used for DTS gap thresholding |
| include/video/detection/litert_engine.h | Add C ABI for LiteRT engine (acquire/release/detect/memory stats) |
| include/video/detection_model_internal.h | Replace TFLite dlopen fields with LiteRT engine handle |
| include/video/api_detection.h | Add frame-arrival timestamp parameter to API detection calls |
| include/database/db_embedded_migrations.h | Add embedded migration 0042 for detection_url |
| include/core/config.h | Add detection_url, grace period, and LiteRT engine config knobs to config |
| db/migrations/0042_add_detection_url.sql | Add migration to introduce detection_url column |
| config/lightnvr.ini | Add LiteRT engine configuration sample block |
| CMakeLists.txt | Build integration for LiteRT submodule and optional delegates |
| .gitmodules | Add LiteRT submodule definition |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+52
to
+59
| [detection_engine] | ||
| ; In-process TFLite/LiteRT inference. When enabled, any stream whose | ||
| ; model_path ends in .tflite will route through this engine. Requires | ||
| ; LightNVR built with -DENABLE_LITERT=ON. Labels are read from the model's | ||
| ; embedded TFLite metadata or a sidecar <basename>.labels.txt. | ||
| enabled = false | ||
| threads = 1 ; Interpreter threads (1-16); raise to use multiple CPU cores | ||
| delegate = xnnpack ; xnnpack | gpu | none (gpu requires -DLITERT_WITH_GPU=ON; falls back to CPU otherwise) |
| #include "tensorflow/lite/delegates/gpu/delegate.h" | ||
| #endif | ||
|
|
||
| extern config_t g_config; |
Comment on lines
+816
to
+825
| // Parse secondary detection stream URL | ||
| cJSON *detection_url_post = cJSON_GetObjectItem(stream_json, "detection_url"); | ||
| if (detection_url_post && cJSON_IsString(detection_url_post)) { | ||
| if (!is_allowed_detection_url(detection_url_post->valuestring)) { | ||
| log_error("Rejected detection_url with disallowed scheme for stream %s", | ||
| config.name); | ||
| cJSON_Delete(stream_json); | ||
| http_response_set_json_error(res, 400, | ||
| "detection_url must use http, https, rtsp, or rtsps"); | ||
| return; |
- CMakeLists: auto-disable LiteRT with a warning instead of FATAL_ERROR when the third_party/litert submodule is absent, so a default configure still builds out of the box; correct the stale "Default OFF" doc comment. - litert_engine.cc: drop the duplicate `extern config_t g_config` declaration -- config.h already declares it with C linkage, so the C++ redeclaration conflicted. - unified_detection_thread: add `rtsps` to the detection-stream protocol whitelist so rtsps:// URLs (accepted by API validation) aren't rejected by FFmpeg. - api_handlers_streams_get: emit `error_message` only when the effective status is "Error", so a stale last_error_message can't linger in a Running/Reconnecting tooltip. - config.c / lightnvr.ini: document that labels come from a sidecar <basename>.labels.txt only (embedded-metadata reading is still a TODO); fix the threads default comment (1). - tests: cover detection_url scheme validation -- POST persists an allowed scheme, and both POST and PUT reject a disallowed (file://) scheme with 400. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Thanks for the review! Addressed the feedback in a single follow-up commit (554ca95, fix(detection): address PR review feedback): Build / correctness
Behavior
Docs
Tests
Not changed: the per-inference timing log in litert_engine_detect() was kept at INFO intentionally to match existing api-based detection behaviour. |
Comment on lines
+431
to
+434
| stream_state_manager_t *sm_err = get_stream_state_by_name(config.name); | ||
| if (sm_err && sm_err->last_error_message[0] != '\0' && strcmp(status, "Error") == 0) { | ||
| cJSON_AddStringToObject(stream_obj, "error_message", sm_err->last_error_message); | ||
| } |
Comment on lines
+591
to
+594
| stream_state_manager_t *sm_err2 = get_stream_state_by_name(config.name); | ||
| if (sm_err2 && sm_err2->last_error_message[0] != '\0' && strcmp(status, "Error") == 0) { | ||
| cJSON_AddStringToObject(stream_obj, "error_message", sm_err2->last_error_message); | ||
| } |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Comment on lines
+816
to
+831
| // Parse secondary detection stream URL | ||
| cJSON *detection_url_post = cJSON_GetObjectItem(stream_json, "detection_url"); | ||
| if (detection_url_post && cJSON_IsString(detection_url_post)) { | ||
| if (!is_allowed_detection_url(detection_url_post->valuestring)) { | ||
| log_error("Rejected detection_url with disallowed scheme for stream %s", | ||
| config.name); | ||
| cJSON_Delete(stream_json); | ||
| http_response_set_json_error(res, 400, | ||
| "detection_url must use http, https, rtsp, or rtsps"); | ||
| return; | ||
| } | ||
| safe_strcpy(config.detection_url, detection_url_post->valuestring, | ||
| sizeof(config.detection_url), 0); | ||
| } else { | ||
| config.detection_url[0] = '\0'; | ||
| } |
Comment on lines
+596
to
+600
| // Surface the specific cause of an Error state. | ||
| stream_state_manager_t *sm_err2 = get_stream_state_by_name(config.name); | ||
| if (sm_err2 && sm_err2->last_error_message[0] != '\0' && strcmp(status, "Error") == 0) { | ||
| cJSON_AddStringToObject(stream_obj, "error_message", sm_err2->last_error_message); | ||
| } |
Comment on lines
+1521
to
+1538
| if (detection_url_put && cJSON_IsString(detection_url_put)) { | ||
| if (!is_allowed_detection_url(detection_url_put->valuestring)) { | ||
| log_error("Rejected detection_url with disallowed scheme for stream %s", | ||
| config.name); | ||
| cJSON_Delete(stream_json); | ||
| http_response_set_json_error(res, 400, | ||
| "detection_url must use http, https, rtsp, or rtsps"); | ||
| return; | ||
| } | ||
| if (strncmp(config.detection_url, detection_url_put->valuestring, | ||
| sizeof(config.detection_url) - 1) != 0) { | ||
| safe_strcpy(config.detection_url, detection_url_put->valuestring, | ||
| sizeof(config.detection_url), 0); | ||
| has_detection_url = true; | ||
| config_changed = true; | ||
| non_dynamic_config_changed = true; | ||
| log_info("Detection URL changed for stream %s", config.name); | ||
| } |
Comment on lines
+435
to
+440
| // Surface the specific cause of an Error state (e.g. failed to load | ||
| // a detection model) so the UI can show it in a tooltip. | ||
| stream_state_manager_t *sm_err = get_stream_state_by_name(config.name); | ||
| if (sm_err && sm_err->last_error_message[0] != '\0' && strcmp(status, "Error") == 0) { | ||
| cJSON_AddStringToObject(stream_obj, "error_message", sm_err->last_error_message); | ||
| } |
| } | ||
| double inference_ms = std::chrono::duration<double, std::milli>( | ||
| std::chrono::steady_clock::now() - t_start).count(); | ||
| log_info("LiteRT: inference %.1f ms (%s)", inference_ms, e->canonical_path.c_str()); |
Contributor
|
@pi-zi thanks for your contribution, I got the CI passing green again and there is a new release version building now that includes these changes. |
themactep
added a commit
to themactep/thingino-firmware
that referenced
this pull request
Jun 26, 2026
Update lightnvr from 4a2a245 to 2335e3f Hash change: 4a2a245bbb2df116eda8f365aafb0b9ab3790b10 -> 2335e3f5de8b1a5aec45548759e4f87e8f765214 Changelog: 0c5c4de: fix(onvif): recognize env:Body namespace prefix in SOAP responses fa948ba: fix(js): resolve CodeQL quality warnings in frontend JS a464197: feat(detection): store optional detection_url per stream 4f91aed: feat(api): accept and emit detection_url on stream endpoints e58163c: feat(detection): rework unified detection thread with a secondary detection stream e9214ea: feat(web): detection URL field in stream config + i18n fd1bb84: feat(detection): make the detection grace period configurable 6e68552: feat(settings): expose detection grace period in the settings UI + i18n adeca19: feat(detection): record detections at frame-arrival time dc8fa50: chore(deps-dev): bump form-data 5e6e2fe: Merge pull request #442 from opensensor/dependabot/npm_and_yarn/web/npm_and_yarn-650b74d069 1ed572c: feat(detection): in-process LiteRT/TFLite detection engine 084c44d: feat(system): report in-process LiteRT detector memory in system stats 5bd06d5: fix(config): don't reload config on settings save (fixes use-after-free) f78a926: fix(log): don't print the detection stream name twice 368d581: @pi-zi has signed the CLA in opensensor/lightNVR#443 554ca95: fix(detection): address PR review feedback 7e48f18: Potential fix for pull request finding 61d21db: Merge pull request #443 from pi-zi/detection-rework 1d8d471: fix: address Copilot review findings from detection-rework merge 9aa6f67: 0.35.4 4359235: fix(build): set CMAKE_POLICY_VERSION_MINIMUM=3.5 for CMake 4.x compat 2335e3f: fix(build): suppress GCC 15 attribute-after-declarator error in XNNPACK
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This branch reworks the detection pipeline on top of 0.35.3 (11 commits, ~+2.4k/−0.7k across 51 files). The headline changes:
The history is structured as a reviewable chain — each commit is self-contained and builds; reviewing commit-by-commit is recommended.
Features
Secondary detection stream (detection_url)
In-process LiteRT/TFLite engine
Configurable grace period
Frame-accurate timestamps
Fixes
Platform notes — XNNPACK on aarch64 big.LITTLE
The XNNPACK delegate is built without hand-written assembly microkernels (XNNPACK_ENABLE_ASSEMBLY=OFF). XNNPACK's cortex_a53 assembly f32 GEMM kernel corrupts an A-row pointer under a multi-threaded pthreadpool on aarch64 big.LITTLE, causing a SIGSEGV with threads > 1 (root-caused from a core dump; reproduces even at XNNPACK HEAD). The portable NEON-intrinsic kernels are stable and multi-thread correctly, at a small single-thread perf cost. ruy remains enabled as the fallback GEMM for ops the delegate doesn't handle.
Build & configuration
Testing